Skip to content

[Backport 2026.1] feature(nemesis): Add precheck() hook to NemesisBaseClass - #15699

Open
scylladbbot wants to merge 5 commits into
scylladb:branch-2026.1from
scylladbbot:backport/15124/to-2026.1
Open

[Backport 2026.1] feature(nemesis): Add precheck() hook to NemesisBaseClass#15699
scylladbbot wants to merge 5 commits into
scylladb:branch-2026.1from
scylladbbot:backport/15124/to-2026.1

Conversation

@scylladbbot

Copy link
Copy Markdown

Implement the skip plan docs/plans/nemesis/nemesis-precheck.md. The precheck will be evaluated before execution, nemesis will be removed from the pool and sent as skipped to Argus (maybe we can in the future add a different mark to distinguish them).

Not currently present, but I will migrate some nemesis to use precheck here for verification. Rest is going to be done separately

Testing

PR pre-checks (self review)

  • I added the relevant backport labels
  • I didn't leave commented-out/debugging code

Reminders

  • Add New configuration option and document them (in sdcm/sct_config.py)

  • Add unit tests to cover my changes (under unit-test/ folder)

  • Update the Readme/doc folder relevant to this change (if needed)

  • (cherry picked from commit 5835f99)

  • (cherry picked from commit d98f803)

  • (cherry picked from commit 964eceb)

  • (cherry picked from commit 82b64e2)

Parent PR: #15124

@scylladbbot

Copy link
Copy Markdown
Author

@pehala - This PR has conflicts, therefore it was moved to draft
Please resolve them and mark this PR as ready for review by removing the conflicts label

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Skill Quality Review

SkillForge (lint + security)

All 1 skills passed lint and security checks

Claude Quality Judge

Failed — see workflow logs

pehala added 4 commits August 11, 2026 14:54
Introduce the return-based precheck() -> str | None feasibility hook on
NemesisBaseClass. Returning None keeps the nemesis runnable; returning a
human-readable reason permanently excludes it from the rotation.

The default implementation returns None, so this is a pure additive extension
point with zero behavioral impact. The docstring documents the contract: use
precheck() only for static conditions (config/backend/edition, Scylla
version/feature flags/cluster-uniform attributes), evaluate cluster-wide probes
against self.runner.cluster.nodes[0] since no target node exists yet, and keep
dynamic-state checks in disrupt().

(cherry picked from commit 5835f99)
… the run loop

Add NemesisRunner.precheck_nemesis(), invoked once at the start of run() before
the execution loop. It iterates disruptions_list, evaluates each nemesis's
precheck(), prunes the infeasible ones in place, and returns the (name, reason)
exclusions. Each exclusion is reported exactly once:
- a returned reason -> DisruptionEvent.skip() (SKIPPED, NORMAL) and Argus
  finalize_nemesis(status=SKIPPED)
- a raised exception -> DisruptionEvent.add_simple_error() (FAILED, ERROR) and
  Argus finalize_nemesis(status=FAILED) -- a broken precheck() is a test error,
  not an intentional skip

Argus reporting reuses argus_submit(), which gains an optional node parameter so
build-time callers can pass cluster.nodes[0] as the representative node (no
target_node exists yet); existing call-sites keep the self.target_node default.

run() calls precheck_nemesis() before the loop and, when exclusions emptied the
rotation, publishes one CRITICAL InfoEvent naming each excluded nemesis and
returns. build_disruptions_by_selector() stays a pure builder returning a plain
list, so runners that legitimately keep an empty disruptions_list and override
call_next_nemesis() (NoOpMonkey, ManagerRcloneBackup, ManagerNativeBackup,
CategoricalMonkey) are unaffected.

Tests: add precheck() stubs to TestBaseClass and TestExecuteBaseClass (required
to keep run()-based tests green now that run() calls precheck()), add
PrecheckSkipNemesis/PrecheckErrorNemesis, and add execute_nemesis/test_precheck.py
covering pruning, DisruptionEvent shapes, Argus submit/finalize, and the run()
empty-rotation path (skip-vs-exception variants parametrized, full event dicts
asserted).

(cherry picked from commit d98f803)
Add a 'Pre-execution skip checks (precheck)' section to docs/nemesis.md covering
the contract, what belongs in precheck() vs disrupt() (plain language, no
internal Category taxonomy), the representative-node rule (cluster.nodes[0]),
the SKIPPED/FAILED reporting behavior, the empty-rotation CRITICAL behavior, and
a before/after migration example. Update the build_disruptions_by_selector runner
example accordingly.

Update the writing-nemesis skill and the AGENTS.md nemesis section with the
precheck() contract and category rule. Advance the plan status to in_progress,
rewrite its Commit Plan section to match the final precheck_nemesis() design,
and regenerate the progress roadmap asset.

(cherry picked from commit 964eceb)
@pehala
pehala force-pushed the backport/15124/to-2026.1 branch from 591d5a1 to 859ffd6 Compare August 11, 2026 12:57
@pehala pehala removed the conflicts label Aug 11, 2026
@pehala
pehala marked this pull request as ready for review August 11, 2026 12:58
@pehala
pehala requested a review from fruch as a code owner August 11, 2026 12:58
@pehala pehala added test-provision-aws Run provision test on AWS test-integration Enable running the integration tests suite labels Aug 11, 2026
branch-2026.1 is behind master on test infra: events_function_scope here
is EventsUtilsMixin (published_events only), not master's FakeEventsDevice
(which also has get_events_by_category()); and TestNemesisClass still
lives in test_sisyphus.py here rather than unit_tests/unit/nemesis/__init__.py.
The cherry-picked tests referenced the master-only APIs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@pehala

pehala commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Integration tests are failing, but they are unrelated. @fruch I think this is safe to merge

11:35:51  FAILED unit_tests/test_config.py::test_20_user_data_format_version_aws - ValueError: AMIs for scylla_version='master:latest' not found in eu-west-1 arch=x86_64

11:35:51  FAILED unit_tests/test_config_get_version_based_on_conf.py::test_images[aws-master] - ValueError: AMIs for scylla_version='master:latest' not found in eu-west-1 arch=x86_64

11:35:51  FAILED unit_tests/test_config_get_version_based_on_conf.py::test_images[gce-master] - assert False == True

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-assisted force_on_cloud P1 Urgent test-integration Enable running the integration tests suite test-provision-aws Run provision test on AWS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants